Fixes #36823 - Create job invocations detail page with main info #839

kmalyjur · 2023-10-11T12:09:38Z

The commit creates a new job invocation detail page (see discussion) which is experimental until finished - you can find it in Lab Features or see /experimental/job_invocations_detail/${JOB_ID}. It includes the main information.

Update: There is a link on the old detail page to the new one if the experimental labs setting is on.

MariaAga · 2023-10-12T16:39:10Z

webpack/JobInvocationDetail/JobInvocationSelectors.js

+import { selectAPIResponse } from 'foremanReact/redux/API/APISelectors';
+
+export const selectItems = state =>
+  selectAPIResponse(state, 'JOB_INVOCATION_KEY') || [];


since items is used as an object { description, status_label: statusLabel, task } = items;, the fallback should be an empty object {}

Actually, selectAPIResponse already gives a fallback, so no fallback needed.
export const selectAPIResponse = (state, key) => selectAPIByKey(state, key).response || {};

MariaAga · 2023-10-12T16:39:21Z

lib/foreman_remote_execution/engine.rb

@@ -232,6 +232,7 @@ class Engine < ::Rails::Engine
          caption: N_('Job templates'),
          parent: :hosts_menu,
          after: :provisioning_templates
+


extra space :)

MariaAga · 2023-10-12T16:42:00Z

webpack/JobInvocationDetail/index.js

+  const finished = statusLabel === 'failed' || statusLabel === 'succeeded';
+  const autoRefresh = task?.state === 'pending' || false;


please use constants for failed, succeeded, pending

MariaAga · 2023-10-12T16:44:19Z

webpack/JobInvocationDetail/index.js

+  const getData = url =>
+    withInterval(
+      get({
+        key: JOB_INVOCATION_KEY,
+        url,
+        handleError: () => {
+          dispatch(stopInterval(JOB_INVOCATION_KEY));
+        },
+      }),
+      1000
+    );
+


Please move to an actions file

MariaAga · 2023-10-16T12:39:38Z

webpack/JobInvocationDetail/JobInvocationOverview.js

+        </DescriptionListDescription>
+      </DescriptionListGroup>
+      <DescriptionListGroup>
+        <DescriptionListTerm>{__('Run on:')}</DescriptionListTerm>


Probably should change the text according to if it already ran or if its going to run in the future, can be done in the next pr but would be nice to have some todo note

MariaAga · 2023-10-16T12:58:39Z

webpack/JobInvocationDetail/JobInvocationOverview.js

+    ssh_user: sshUser,
+    template_invocations: template,
+  } = data;
+  const effectiveUser = ssh ? ssh.effective_user : null;


When setting an effective user on a job, I dont see its value in the job invocation

kmalyjur · 2023-10-24T11:32:22Z

@MariaAga Thank you for your review, I've updated the code and changed how I retrieve "effective_user" and some other attributes.

kmalyjur · 2023-10-25T14:28:25Z

Thanks to Adam's feedback, I've just deleted my unnecessary changes in the job_invocations_controller.rb.

MariaAga · 2023-10-25T09:53:53Z

webpack/JobInvocationDetail/JobInvocationSelectors.js

+import { selectAPIResponse } from 'foremanReact/redux/API/APISelectors';
+
+export const selectItems = state =>
+  selectAPIResponse(state, 'JOB_INVOCATION_KEY');


JOB_INVOCATION_KEY should be from JobInvocationConstants

MariaAga · 2023-10-31T11:39:46Z

webpack/JobInvocationDetail/JobInvocationOverview.js

+  const dateConverted = new Date(startAt);
+  const dateLocaleFormatted = dateConverted
+    .toLocaleString(documentLocale(), dateOptions)
+    .replace(/(\d{4}) /, '$1, ');


why have this line? it adds a , after the year but in some locales it doesnt make sense to add.

I wanted to do exactly what you said, but I didn't realize it may be strange in other locales. I'll just delete it

webpack/JobInvocationDetail/JobInvocationOverview.js

MariaAga

Thanks, LGTM!
@adamruzicka Can you take a look at the ruby parts?

lib/foreman_remote_execution/engine.rb

webpack/JobInvocationDetail/JobInvocationOverview.js

lib/foreman_remote_execution/engine.rb

app/views/api/v2/job_invocations/base.json.rabl

adamruzicka

One last nitpick, apart from it lgtm

app/views/api/v2/job_invocations/base.json.rabl

adamruzicka

Looks good

adamruzicka · 2023-11-29T10:47:13Z

Thank you @kmalyjur & @MariaAga !

pr-processor bot added Not yet reviewed Waiting on contributor labels Oct 11, 2023

kmalyjur closed this Oct 11, 2023

kmalyjur reopened this Oct 11, 2023

MariaAga reviewed Oct 12, 2023

View reviewed changes

MariaAga reviewed Oct 16, 2023

View reviewed changes

kmalyjur force-pushed the job-inv-main branch from 9eac59a to d7b7cb8 Compare October 23, 2023 14:00

pr-processor bot removed the Waiting on contributor label Oct 23, 2023

kmalyjur force-pushed the job-inv-main branch 2 times, most recently from c07d707 to 6c1f9d2 Compare October 24, 2023 11:23

kmalyjur force-pushed the job-inv-main branch from 6c1f9d2 to fee84c3 Compare October 25, 2023 14:24

MariaAga suggested changes Oct 31, 2023

View reviewed changes

pr-processor bot added Waiting on contributor and removed Not yet reviewed labels Oct 31, 2023

kmalyjur force-pushed the job-inv-main branch from fee84c3 to 81a1fff Compare November 6, 2023 13:22

pr-processor bot added Needs re-review and removed Waiting on contributor labels Nov 6, 2023

kmalyjur force-pushed the job-inv-main branch 2 times, most recently from 347c92e to 6dd5986 Compare November 6, 2023 14:30

MariaAga approved these changes Nov 7, 2023

View reviewed changes

pr-processor bot removed the Needs re-review label Nov 7, 2023

adamruzicka reviewed Nov 15, 2023

View reviewed changes

kmalyjur force-pushed the job-inv-main branch 2 times, most recently from 8de6792 to 8e6a585 Compare November 22, 2023 13:17

kmalyjur force-pushed the job-inv-main branch from 8e6a585 to 16f7d43 Compare November 28, 2023 17:05

adamruzicka reviewed Nov 29, 2023

View reviewed changes

app/views/api/v2/job_invocations/base.json.rabl Outdated Show resolved Hide resolved

Fixes #36823 - Create job invocations detail page with main info

5112090

kmalyjur force-pushed the job-inv-main branch from 16f7d43 to 5112090 Compare November 29, 2023 10:06

kmalyjur requested a review from adamruzicka November 29, 2023 10:06

adamruzicka approved these changes Nov 29, 2023

View reviewed changes

adamruzicka merged commit ea8a0d6 into theforeman:master Nov 29, 2023
5 checks passed

kmalyjur mentioned this pull request Feb 5, 2024

Fixes #37122 - Update system status chart in job invocations detail page #871

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #36823 - Create job invocations detail page with main info #839

Fixes #36823 - Create job invocations detail page with main info #839

kmalyjur commented Oct 11, 2023 •

edited

Loading

MariaAga Oct 12, 2023

MariaAga Oct 12, 2023

MariaAga Oct 12, 2023

MariaAga Oct 12, 2023

MariaAga Oct 12, 2023

MariaAga Oct 16, 2023

MariaAga Oct 23, 2023

MariaAga Oct 16, 2023

kmalyjur commented Oct 24, 2023

kmalyjur commented Oct 25, 2023

MariaAga Oct 25, 2023

MariaAga Oct 31, 2023

kmalyjur Nov 2, 2023

MariaAga left a comment

adamruzicka left a comment

adamruzicka left a comment

adamruzicka commented Nov 29, 2023

		const finished = statusLabel === 'failed' \|\| statusLabel === 'succeeded';
		const autoRefresh = task?.state === 'pending' \|\| false;

Fixes #36823 - Create job invocations detail page with main info #839

Fixes #36823 - Create job invocations detail page with main info #839

Conversation

kmalyjur commented Oct 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kmalyjur commented Oct 24, 2023

kmalyjur commented Oct 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MariaAga left a comment

Choose a reason for hiding this comment

adamruzicka left a comment

Choose a reason for hiding this comment

adamruzicka left a comment

Choose a reason for hiding this comment

adamruzicka commented Nov 29, 2023

kmalyjur commented Oct 11, 2023 •

edited

Loading